fix(webapp): Public links says "Loading…", not a premature "no" (BEA-78) - #127
Open
ssowonny wants to merge 1 commit into
Open
fix(webapp): Public links says "Loading…", not a premature "no" (BEA-78)#127ssowonny wants to merge 1 commit into
ssowonny wants to merge 1 commit into
Conversation
The public-links panel is where a member answers "is anything of ours public right now?". SharesTable read an empty array as a settled answer, and both callers hand it `shares || []` while the request is still in flight — so the panel printed "No public links." as a confident NO, then swapped to a table listing an active, never-expiring link. Fix at the choke point: an optional `loading` prop honored above the empty branch, in the same .admin-list/.admin-empty shell so the section is one row tall either way and doesn't jump when the data lands. Wired at both call sites — project Settings and the org-wide cross-project audit, which had the identical bug. `isLoading`, not `isPending`: OrgAdmin's shares query is `enabled: owner`, and TanStack reports isPending true forever for a disabled query. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TL;DR
SharesTable, not either caller: an empty array read as a settled answer. Oneloadingprop, honored above the empty branch.isLoading, notisPending—OrgAdmin's shares query isenabled: owner, and a disabled query pinsisPendingtrue forever.Closes BEA-78.
What changed
Once the response lands, both are the same table — the fix only replaces the frame where the panel was lying:
Three files, one idea each:
SharesTable.tsx—loading?: boolean, branch aboveshares.length === 0, same.admin-list/.admin-emptyshell so the section is one row tall in either state and doesn't jump when the data arrives.ProjectSettings.tsx—PublicLinksdestructuresisLoadingoffuseSharesand passes it. Theif (error) return nullguard is untouched, so a failed shares request still hides the section entirely.OrgAdmin.tsx— same, off its ownuseQueryfor/api/orgs/<id>/shares.ShareBanneris unchanged on purpose: it returnsnullon an empty list, and an absent banner makes no claim.Why
isLoadingThe spec said
isPending; the plan flagged it and this PR follows the plan.OrgAdmin.tsxsetsenabled: owneron the shares query, and TanStack reportsisPending: truefor a disabled query — a non-owner would get a permanent skeleton. The section happens to sit inside{owner && …}today, so nothing would be seen, but that is incidental protection.isLoading(pending and fetching) costs nothing and makes it impossible.What was run
go build ./...,go vet ./...,go test ./...— all packages pass.npm run e2e— 154 passed, 1 skipped (the skip is the pre-existingsec14fetest.skip).browse.spec.ts:368throttles**/api/p/*/sharesby 2s, asserts the loading row is visible and "No public links." has count 0 while in flight, then asserts the shared row appears. Verified it fails on the unfixed bundle — reverted the prop, rebuilt, watched it go red, restored.npm run buildininternal/webapp/frontend; rebuiltinternal/webapp/staticis committed andcheck-dist.shshows only the expected asset rotation.No architecture diagram change:
SharesTableappears inwebapp-frontend.mdas a name inside the components namespace, with no props or relationships drawn, and an optional prop changes neither.The one loose end
BEA-79 moves this exact call site when it reorders the Settings sections. This lands first (it's the older issue), so that move applies on top — but whoever takes BEA-79 should expect the
static/bundle to conflict, and resolve it by re-runningnpm run build, never by picking a side.Build session
(Only works on the machine that ran the build.)